[standards] ReactPrivate, an explicit interface between the renderer and RN#24782
Closed
ide wants to merge 2 commits into
Closed
[standards] ReactPrivate, an explicit interface between the renderer and RN#24782ide wants to merge 2 commits into
ide wants to merge 2 commits into
Conversation
Closed
12 tasks
analysis-bot
suggested changes
May 9, 2019
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
36ad95e to
871bfa6
Compare
analysis-bot
suggested changes
May 9, 2019
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
871bfa6 to
4b03978
Compare
ide
added a commit
to ide/react
that referenced
this pull request
May 10, 2019
…derer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
ide
added a commit
to ide/react
that referenced
this pull request
May 10, 2019
…derer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
4b03978 to
5fc65c7
Compare
analysis-bot
suggested changes
May 10, 2019
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
ide
added a commit
to ide/react
that referenced
this pull request
May 10, 2019
…derer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
622fcb2 to
49fe18b
Compare
analysis-bot
suggested changes
May 10, 2019
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
analysis-bot
suggested changes
May 10, 2019
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
…and RN This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals. Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals. Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste. There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit. Test Plan: Run unit tests, CI. This commit should be safe since it just introduces new modules. Also tested with newly generated renderers (not in this commit; needs to happen in the React repo) that use ReactPrivate instead of Haste and verified that RNTester loads and that unit tests pass.
The ReactPrivate modules are the new forwarding interface between RN and the renderers generated from the React repository. These `lib` files may have been unused since `InitializeJavaScriptAppEngine` is not referenced anywhere in the RN and React repositories.
49fe18b to
67eb355
Compare
Contributor
Author
|
@cpojer -- This is ready for review (tests passing, conforms to the interface Sebastian suggested on the React PR). |
facebook-github-bot
approved these changes
May 20, 2019
Contributor
facebook-github-bot
left a comment
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Collaborator
|
This pull request was successfully merged by @ide in 9cd8825. When will my fix make it into a release? | Upcoming Releases |
ide
added a commit
to ide/react
that referenced
this pull request
May 22, 2019
…derer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
cpojer
pushed a commit
to facebook/react
that referenced
this pull request
May 23, 2019
…derer (#15604) * [react-native] Use path-based imports instead of Haste for the RN renderer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app. * Access natively defined "nativeFabricUIManager" instead of importing it Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in #15604 (review) for more context).
gaearon
pushed a commit
to gaearon/react
that referenced
this pull request
Jun 11, 2019
…derer (facebook#15604) * [react-native] Use path-based imports instead of Haste for the RN renderer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app. * Access natively defined "nativeFabricUIManager" instead of importing it Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in facebook#15604 (review) for more context).
gaearon
pushed a commit
to gaearon/react
that referenced
this pull request
Jun 19, 2019
…derer (facebook#15604) * [react-native] Use path-based imports instead of Haste for the RN renderer To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import. On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface. The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`. Task description: facebook/react-native#24770 Sister RN PR (needs to land before this one): facebook/react-native#24782 Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app. * Access natively defined "nativeFabricUIManager" instead of importing it Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in facebook#15604 (review) for more context).
vovkasm
pushed a commit
to vovkasm/react-native
that referenced
this pull request
Aug 7, 2019
…book#24782) Summary: This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals. Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals. Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste. There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit. ## Changelog [General] [Changed] - Add a private interface (do not use) between the renderer and RN Pull Request resolved: facebook#24782 Differential Revision: D15413477 Pulled By: cpojer fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4 (cherry picked from commit 9cd8825)
aleclarson
pushed a commit
to alloc/react-native-macos
that referenced
this pull request
Nov 2, 2019
Summary: This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals. Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook/react-native#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals. Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste. There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit. ## Changelog [General] [Changed] - Add a private interface (do not use) between the renderer and RN Pull Request resolved: facebook/react-native#24782 Differential Revision: D15413477 Pulled By: cpojer fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
aleclarson
pushed a commit
to alloc/react-native-macos
that referenced
this pull request
Nov 2, 2019
Summary: This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals. Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook/react-native#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals. Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste. There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit. ## Changelog [General] [Changed] - Add a private interface (do not use) between the renderer and RN Pull Request resolved: facebook/react-native#24782 Differential Revision: D15413477 Pulled By: cpojer fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
M-i-k-e-l
pushed a commit
to M-i-k-e-l/react-native
that referenced
this pull request
Mar 10, 2020
…book#24782) Summary: This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals. Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals. Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste. There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit. ## Changelog [General] [Changed] - Add a private interface (do not use) between the renderer and RN Pull Request resolved: facebook#24782 Differential Revision: D15413477 Pulled By: cpojer fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (
ReactNativePrivateInterface) that the renderers use to access RN internals.Motivation: The main goal is to move one step closer to turning off Haste for RN (#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.
Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.
There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.
Changelog
[General] [Changed] - Add a private interface (do not use) between the renderer and RN
Test Plan
Run unit tests, CI. This commit should be safe since it just introduces new modules.
Also tested with newly generated renderers (not in this commit; needs to happen in the React repo) that use ReactPrivate instead of Haste and verified that RNTester loads and that unit tests pass.